refactor: name patch fragments by intent and codify patch naming rules#2474
refactor: name patch fragments by intent and codify patch naming rules#2474devantler wants to merge 2 commits into
Conversation
Patch fragments now follow the CR-folder naming convention: they live under
a patches/ directory, hold one resource per file, and are named
<verb>-<purpose>.yaml — no Kind-led names, no redundant -patch suffix (the
flux-kustomization prefix stays, per the Flux CR rule). Talos machine-config
patches (talos/, talos-local/) are explicitly exempt from all naming and
file-structure conventions. validate-naming.py enforces the new rules
(checks 7-8); three no-op patches (two empty spec: {} placeholders and a
Corefile patch identical to the base) are dropped. All five affected
overlays render byte-identical before and after.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request updates naming guidance, validation logic, and references across docs and Kubernetes overlays to use intent-based patch filenames. It also removes obsolete patch manifests, rewires provider kustomization entries to new patch files, and adds/splits multiple Talos machine-config and network-rule manifests. Talos volume encryption is split into separate state and ephemeral configs, and several comment references are updated to match the new filenames. Sequence Diagram(s)Not applicable. Compact metadata
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Talos machine-config patches now follow the same intent-naming convention as CR folders and k8s patches: <verb>-<purpose>.yaml, one YAML document per file. The multi-document files are split (disk-encryption into its two VolumeConfigs; each role's ingress-firewall into one NetworkRuleConfig per file, preserving the consolidated rule COUNT and the ENOBUFS warning), all non-comment config lines are verified preserved doc-for-doc, and validate-naming.py check 9 enforces the rules in CI. ksail globs the talos*/ directories, so no config reference changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
talos/workers/allow-cilium-wireguard-ingress.yaml (1)
1-15: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider consolidating with
allow-cni-vxlan-ingress.yaml.This rule (udp/51871) and
allow-cni-vxlan-ingress.yaml(udp/8472) share identical protocol (udp) and subnet set (10.0.0.0/16only). Per the repo's own consolidation convention, these should be merged into a single rule'sportslist to minimize total rule count.♻️ Example merge
-# Cilium WireGuard transparent encryption (encryption.type: wireguard). -# Tunnels VXLAN-encapsulated pod traffic between nodes through cilium_wg0. -# Default port is UDP 51871; Talos's NetworkDefaultActionConfig: block -# would otherwise drop inter-node WireGuard packets and break pod-to-pod -# traffic the moment encryption is enabled. +# Cilium VXLAN (8472) and WireGuard transparent encryption (51871, +# encryption.type: wireguard) cluster-internal only on worker nodes. +# Talos's NetworkDefaultActionConfig: block would otherwise drop +# inter-node tunnel traffic. apiVersion: v1alpha1 kind: NetworkRuleConfig -name: cilium-wireguard-ingress +name: cni-tunnel-ingress portSelector: ports: + - 8472 - 51871 protocol: udp ingress: - subnet: 10.0.0.0/16🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@talos/workers/allow-cilium-wireguard-ingress.yaml` around lines 1 - 15, The WireGuard ingress rule duplicates the same udp protocol and subnet allowlist already used by allow-cni-vxlan-ingress.yaml, so consolidate them to reduce rule count. Update the NetworkRuleConfig in cilium-wireguard-ingress to share the same ingress definition and combine the port entries with the existing allow-cni-vxlan-ingress rule, keeping the identifying symbols cilium-wireguard-ingress and portSelector/ingress aligned with the repo’s consolidation pattern.talos/workers/allow-cilium-health-ingress.yaml (1)
1-12: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider consolidating with
allow-cilium-mutual-auth-ingress.yaml.Both this rule (tcp/4240) and
allow-cilium-mutual-auth-ingress.yaml(tcp/4250) share the exact same protocol (tcp) and subnet set (10.0.0.0/16only). Per the repo's own consolidation rule, matching protocol+subnet rules should be merged into one rule'sportslist rather than kept as separateNetworkRuleConfigfiles, to keep the total rule count low.♻️ Example merge
-# Cilium health checks (4240) cluster-internal only on worker nodes. Pairs -# with cluster/block-ingress-by-default.yaml. +# Cilium health checks (4240) and SPIRE mutual-auth handshakes (4250) +# cluster-internal only on worker nodes. Pairs with +# cluster/block-ingress-by-default.yaml. apiVersion: v1alpha1 kind: NetworkRuleConfig -name: cilium-health-ingress +name: cilium-health-and-mutual-auth-ingress portSelector: ports: - 4240 + - 4250 protocol: tcp ingress: - subnet: 10.0.0.0/16If the two rules are kept separate for documentation clarity (the mutual-auth file carries substantial incident-history commentary), it'd be worth a short note explaining the deliberate exception to the consolidation rule.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@talos/workers/allow-cilium-health-ingress.yaml` around lines 1 - 12, The cilium health ingress rule duplicates the same tcp protocol and subnet set used by allow-cilium-mutual-auth-ingress.yaml, so it should be consolidated rather than kept as a separate NetworkRuleConfig. Update the relevant NetworkRuleConfig entries so the shared tcp/10.0.0.0/16 allowance is represented in one rule with both ports in the ports list, and remove the redundant standalone rule; if you intentionally keep them separate for documentation, add a brief note in the cilium-health-ingress or allow-cilium-mutual-auth-ingress file explaining the exception.talos/workers/allow-kubelet-ingress.yaml (1)
1-17: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider consolidating with
allow-hubble-peer-ingress.yaml.This rule (tcp/10250) and
allow-hubble-peer-ingress.yaml(tcp/4244) share identical protocol (tcp) and the exact same two-subnet set (10.0.0.0/16+10.244.0.0/16). Per the repo's stated consolidation convention, these should be merged into one rule'sportslist to keep the total rule count low.♻️ Example merge
-# kubelet (10250) cluster-internal only on worker nodes. Pairs with -# cluster/block-ingress-by-default.yaml. +# Hubble peer (4244) and kubelet (10250) cluster-internal only on worker +# nodes. Pairs with cluster/block-ingress-by-default.yaml. apiVersion: v1alpha1 kind: NetworkRuleConfig -name: kubelet-ingress +name: hubble-peer-and-kubelet-ingress portSelector: ports: + - 4244 - 10250 protocol: tcp ingress: # Node CIDR covers cross-node scrapes: Cilium masquerades pod->node-IP # traffic to the source node IP. Same-node scrapes (e.g. the local # metrics-server replica hitting its own kubelet) are NOT masqueraded, so # the kubelet sees the raw pod IP and the pod CIDR must be allowed too. - subnet: 10.0.0.0/16 - subnet: 10.244.0.0/16🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@talos/workers/allow-kubelet-ingress.yaml` around lines 1 - 17, Merge the kubelet-ingress NetworkRuleConfig with allow-hubble-peer-ingress.yaml by reusing the same ingress subnet set and consolidating the tcp ports into one rule’s ports list, since both rules are identical except for the port. Update the NetworkRuleConfig definitions that currently live in kubelet-ingress and allow-hubble-peer-ingress so the shared subnets remain in a single rule and the portSelector includes both 10250 and 4244. Keep the existing symbol names kubelet-ingress and allow-hubble-peer-ingress as the location anchors while reducing the total number of rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/validate-naming.py`:
- Around line 112-119: The `count_docs` helper in `validate-naming.py` uses the
ambiguous loop variable name `l`, which triggers Ruff’s `E741` lint error.
Rename that inner loop variable in the `any(...)` expression to a clearer name,
keeping the same document-counting logic and preserving the behavior of
`count_docs`.
---
Outside diff comments:
In `@talos/workers/allow-cilium-health-ingress.yaml`:
- Around line 1-12: The cilium health ingress rule duplicates the same tcp
protocol and subnet set used by allow-cilium-mutual-auth-ingress.yaml, so it
should be consolidated rather than kept as a separate NetworkRuleConfig. Update
the relevant NetworkRuleConfig entries so the shared tcp/10.0.0.0/16 allowance
is represented in one rule with both ports in the ports list, and remove the
redundant standalone rule; if you intentionally keep them separate for
documentation, add a brief note in the cilium-health-ingress or
allow-cilium-mutual-auth-ingress file explaining the exception.
In `@talos/workers/allow-cilium-wireguard-ingress.yaml`:
- Around line 1-15: The WireGuard ingress rule duplicates the same udp protocol
and subnet allowlist already used by allow-cni-vxlan-ingress.yaml, so
consolidate them to reduce rule count. Update the NetworkRuleConfig in
cilium-wireguard-ingress to share the same ingress definition and combine the
port entries with the existing allow-cni-vxlan-ingress rule, keeping the
identifying symbols cilium-wireguard-ingress and portSelector/ingress aligned
with the repo’s consolidation pattern.
In `@talos/workers/allow-kubelet-ingress.yaml`:
- Around line 1-17: Merge the kubelet-ingress NetworkRuleConfig with
allow-hubble-peer-ingress.yaml by reusing the same ingress subnet set and
consolidating the tcp ports into one rule’s ports list, since both rules are
identical except for the port. Update the NetworkRuleConfig definitions that
currently live in kubelet-ingress and allow-hubble-peer-ingress so the shared
subnets remain in a single rule and the portSelector includes both 10250 and
4244. Keep the existing symbol names kubelet-ingress and
allow-hubble-peer-ingress as the location anchors while reducing the total
number of rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7e39c7d3-7696-48f8-bdee-9c6afb37c8d4
📒 Files selected for processing (43)
AGENTS.mddocs/dr/spire-server-ha.mddocs/runtime-security.mddocs/rwx-storage.mdk8s/bases/infrastructure/cluster-policies/best-practices/verify-ksail-images.yamlk8s/bases/infrastructure/cluster-security-exceptions/image-verification.yamlk8s/providers/hetzner/infrastructure/cluster-policies/restrict-storage-to-baseline-workers.yamlk8s/providers/hetzner/infrastructure/controllers/cilium/patches/enforce-wireguard-strict-mode.yamlscripts/validate-naming.pytalos-local/cluster/disable-default-cni-and-kube-proxy.yamltalos-local/cluster/enable-dex-oidc.yamltalos/cluster/authenticate-ghcr-pulls.yamltalos/cluster/block-ingress-by-default.yamltalos/cluster/disable-default-cni-and-kube-proxy.yamltalos/cluster/enable-apparmor.yamltalos/cluster/enable-audit-logging.yamltalos/cluster/enable-dex-oidc.yamltalos/cluster/enable-user-namespaces.yamltalos/cluster/encrypt-ephemeral-volume.yamltalos/cluster/encrypt-state-volume.yamltalos/cluster/evict-pods-before-oom.yamltalos/cluster/gc-terminated-pods-sooner.yamltalos/cluster/harden-kernel-sysctls.yamltalos/cluster/use-platform-hostname.yamltalos/cluster/verify-first-party-images.yamltalos/control-planes/allow-internal-node-ingress.yamltalos/control-planes/allow-internal-nodepod-ingress.yamltalos/control-planes/allow-internal-udp-ingress.yamltalos/control-planes/allow-public-ingress.yamltalos/control-planes/ingress-firewall.yamltalos/workers/allow-apid-ingress.yamltalos/workers/allow-cilium-health-ingress.yamltalos/workers/allow-cilium-mutual-auth-ingress.yamltalos/workers/allow-cilium-wireguard-ingress.yamltalos/workers/allow-cni-vxlan-ingress.yamltalos/workers/allow-hubble-peer-ingress.yamltalos/workers/allow-kubelet-ingress.yamltalos/workers/allow-node-exporter-ingress.yamltalos/workers/allow-nodeport-ingress.yamltalos/workers/ingress-firewall.yamltalos/workers/label-nodes.yamltalos/workers/load-kvm-modules.yamltalos/workers/mount-longhorn-data.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
devantler-tech/actions(auto-detected)devantler-tech/aws(auto-detected)devantler-tech/reusable-workflows(auto-detected)devantler-tech/ksail(auto-detected)devantler-tech/ascoachingogvaner(auto-detected)devantler-tech/wedding-app(auto-detected)devantler-tech/agent-skills(auto-detected)
💤 Files with no reviewable changes (2)
- talos/control-planes/ingress-firewall.yaml
- talos/workers/ingress-firewall.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
talos/**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Talos machine-config patches must contain one YAML document per file and use intent-describing
verb-purpose.yamlfilenames; they are Talos config fragments, not Kubernetes manifests.
Files:
talos/workers/allow-cilium-health-ingress.yamltalos/workers/allow-hubble-peer-ingress.yamltalos/workers/allow-kubelet-ingress.yamltalos/workers/allow-node-exporter-ingress.yamltalos/control-planes/allow-internal-udp-ingress.yamltalos/workers/allow-cni-vxlan-ingress.yamltalos/workers/allow-apid-ingress.yamltalos/cluster/enable-dex-oidc.yamltalos/control-planes/allow-internal-nodepod-ingress.yamltalos/workers/allow-cilium-mutual-auth-ingress.yamltalos/cluster/verify-first-party-images.yamltalos/cluster/authenticate-ghcr-pulls.yamltalos/control-planes/allow-public-ingress.yamltalos/cluster/encrypt-state-volume.yamltalos/control-planes/allow-internal-node-ingress.yamltalos/workers/allow-cilium-wireguard-ingress.yamltalos/cluster/gc-terminated-pods-sooner.yamltalos/workers/allow-nodeport-ingress.yamltalos/workers/mount-longhorn-data.yamltalos/cluster/encrypt-ephemeral-volume.yaml
k8s/bases/**
📄 CodeRabbit inference engine (AGENTS.md)
Do not modify base manifests under
k8s/bases/directly from overlays; make changes via Kustomizepatches:in overlays instead.
Files:
k8s/bases/infrastructure/cluster-security-exceptions/image-verification.yamlk8s/bases/infrastructure/cluster-policies/best-practices/verify-ksail-images.yaml
k8s/**
📄 CodeRabbit inference engine (AGENTS.md)
k8s/**: Keep Kubernetes manifests organized as a GitOps/Kustomize repository: use hierarchical base → provider overlay → cluster overlay structure, and respect Flux dependency order (bootstrap→infrastructure-controllers→infrastructure→apps).
For Kubernetes resources, use one resource per file, keep directories kebab-case, and name files after the resource Kind (with documented exceptions for CR folders andpatches/folders).
FluxKustomizationCR manifests must be namedflux-kustomization*.yaml, while the Kustomize build file must remain exactlykustomization.yaml.
Files:
k8s/bases/infrastructure/cluster-security-exceptions/image-verification.yamlk8s/bases/infrastructure/cluster-policies/best-practices/verify-ksail-images.yamlk8s/providers/hetzner/infrastructure/controllers/cilium/patches/enforce-wireguard-strict-mode.yamlk8s/providers/hetzner/infrastructure/cluster-policies/restrict-storage-to-baseline-workers.yaml
k8s/bases/infrastructure/**
📄 CodeRabbit inference engine (AGENTS.md)
k8s/bases/infrastructure/**: Underk8s/bases/infrastructure/, use the component-folder-first layout: keep a component's HelmRelease/HelmRepository and its own CRs together in a folder named after the component, and split CRs into plural-Kind folders only for dependency or cluster-scoped reasons.
Place cluster-scoped or cross-cutting infrastructure resources in the appropriate plural-Kind folders (for examplecluster-policies/,cluster-roles/,external-secrets/,limit-ranges/,vertical-pod-autoscalers/).
Files:
k8s/bases/infrastructure/cluster-security-exceptions/image-verification.yamlk8s/bases/infrastructure/cluster-policies/best-practices/verify-ksail-images.yaml
k8s/**/patches/**
📄 CodeRabbit inference engine (AGENTS.md)
Patch fragments must live under a
patches/directory, use intent-describing filenames (verb-purpose.yaml), avoid a-patchsuffix, and still follow one-resource-per-file.
Files:
k8s/providers/hetzner/infrastructure/controllers/cilium/patches/enforce-wireguard-strict-mode.yaml
🧠 Learnings (2)
📚 Learning: 2026-07-01T21:13:36.950Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 2359
File: k8s/bases/apps/actual-budget/helm-release.yaml:62-111
Timestamp: 2026-07-01T21:13:36.950Z
Learning: When reviewing Kustomize/Helm YAML in this repo, keep the base vs provider overlay split: `k8s/bases/apps/**` and `k8s/bases/infrastructure/**` should contain each app’s full, environment-agnostic configuration (including base-level postRenderer Kustomize patches such as deployment strategy, topology spread, probes, and env injection). `k8s/providers/{docker,hetzner}/**` should only add small provider-specific deltas (e.g., `interval`, `persistence.size`) via patch files (like `k8s/providers/<provider>/apps/<app>/patches/helm-release-patch.yaml`). If configuration is identical across providers (e.g., OIDC/OAuth env vars where `${domain}` is resolved per cluster via envsubst), it belongs in the base and must not be duplicated into provider overlays.
Applied to files:
k8s/bases/infrastructure/cluster-security-exceptions/image-verification.yamlk8s/bases/infrastructure/cluster-policies/best-practices/verify-ksail-images.yamlk8s/providers/hetzner/infrastructure/controllers/cilium/patches/enforce-wireguard-strict-mode.yamlk8s/providers/hetzner/infrastructure/cluster-policies/restrict-storage-to-baseline-workers.yaml
📚 Learning: 2026-07-04T13:30:04.759Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 2446
File: k8s/bases/infrastructure/cluster-security-exceptions/delete-rbac.yaml:38-125
Timestamp: 2026-07-04T13:30:04.759Z
Learning: For Kubescape ClusterSecurityException (apiVersion kubescape.io/v1beta1) and the mirrored Headlamp exception config, do NOT pin `spec.match.resources[].name` (and Headlamp `attributes.name`) to a single literal value when the identifier includes a generated hash. These fields are compared using `regexCompare`, so match such resources with an anchored regular expression that covers the stable prefix and the hash pattern (e.g., `^crossplane:provider:provider-upjet-github-[0-9a-f]+:system$`) rather than the current hash, so the exception remains valid across provider re-installs/revisions.
Applied to files:
k8s/bases/infrastructure/cluster-security-exceptions/image-verification.yaml
🪛 ast-grep (0.44.1)
scripts/validate-naming.py
[warning] 113-113: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(path, encoding="utf-8", errors="replace")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 Ruff (0.15.20)
scripts/validate-naming.py
[error] 118-118: Ambiguous variable name: l
(E741)
[warning] 174-174: Use list.extend to create a transformed list
(PERF401)
🔀 Multi-repo context devantler-tech/ksail, devantler-tech/actions, devantler-tech/aws, devantler-tech/reusable-workflows, devantler-tech/ascoachingogvaner, devantler-tech/wedding-app, devantler-tech/agent-skills
Linked repositories findings
devantler-tech/ksail
docs/src/content/docs/guides/talos-native-patches.mdxstill documents deprecated/generated Talos patch filenames astalos/cluster/image-verification.yaml,talos/cluster/disable-cdi.yaml, and ingress firewall filestalos/cluster/ingress-firewall-default-action.yaml,talos/control-planes/ingress-firewall-rules.yaml,talos/workers/ingress-firewall-rules.yaml. Those examples are now inconsistent with the PR’s renamed intent-based patch files and Talos file split conventions. [::devantler-tech/ksail::]docs/src/content/docs/distributions/talos.mdxstill shows the old illustrative layout withdisable-default-cni.yaml,image-verification.yaml,node-labels.yaml,storage.yaml, andingress-firewall-rules.yaml; this is relevant because the PR updates naming guidance and some actual Talos filenames (for example the new default CNI disable file and renamed Talos patches). [::devantler-tech/ksail::]pkg/fsutil/generator/talos/generator.go,pkg/fsutil/scaffolder/scaffolder_talos.go, and related tests still referenceimage-verification.yamlanddisable-default-cni.yamldirectly. If the PR’s naming changes are meant to affect generated/scaffolded Talos files, these are the primary code-path references to reconcile. [::devantler-tech/ksail::]
Other repositories
- No relevant matches were found in
devantler-tech/actions,devantler-tech/aws,devantler-tech/reusable-workflows,devantler-tech/ascoachingogvaner,devantler-tech/wedding-app, ordevantler-tech/agent-skills. [::devantler-tech/actions::] [::devantler-tech/aws::] [::devantler-tech/reusable-workflows::] [::devantler-tech/ascoachingogvaner::] [::devantler-tech/wedding-app::] [::devantler-tech/agent-skills::]
🔇 Additional comments (28)
AGENTS.md (2)
209-221: 📐 Maintainability & Code QualityNaming-convention text matches guidelines; flag cross-repo drift.
The rewritten rules correctly match the
talos/**/*.yamlguideline (one doc per file, intent-basedverb-purpose.yamlnames, k8s-specific rules exempted) and thek8s/**/patches/**guideline (patches/ placement, no-patchsuffix).However, linked-repo research on
devantler-tech/ksailshows its docs (talos-native-patches.mdx,talos.mdx) and generator/scaffolder code still reference deprecated names likeimage-verification.yaml,disable-default-cni.yaml, andingress-firewall-rules.yaml, which conflict with the intent-based, one-doc-per-file conventions formalized here. If ksail scaffolds clusters using this repo's conventions, those references should be reconciled.Source: Linked repositories
259-259: LGTM!docs/dr/spire-server-ha.md (1)
128-128: LGTM!docs/runtime-security.md (1)
27-33: LGTM!docs/rwx-storage.md (1)
73-77: LGTM!talos/control-planes/allow-internal-nodepod-ingress.yaml (1)
1-23: LGTM!talos/control-planes/allow-internal-udp-ingress.yaml (1)
1-21: LGTM!talos/control-planes/allow-public-ingress.yaml (1)
1-27: LGTM!k8s/bases/infrastructure/cluster-policies/best-practices/verify-ksail-images.yaml (1)
6-6: LGTM!k8s/bases/infrastructure/cluster-security-exceptions/image-verification.yaml (1)
6-6: LGTM!k8s/providers/hetzner/infrastructure/controllers/cilium/patches/enforce-wireguard-strict-mode.yaml (1)
84-84: LGTM!Also applies to: 101-101
talos/cluster/verify-first-party-images.yaml (1)
36-36: LGTM!talos/control-planes/allow-internal-node-ingress.yaml (1)
1-27: LGTM!talos/cluster/gc-terminated-pods-sooner.yaml (1)
22-22: 🎯 Functional Correctness
talos/cluster/enable-audit-logging.yamlalready exists.> Likely an incorrect or invalid review comment.k8s/providers/hetzner/infrastructure/cluster-policies/restrict-storage-to-baseline-workers.yaml (1)
7-7: 🎯 Functional CorrectnessDrop this comment
talos/workers/label-nodes.yamlexists, so the path is not dangling.> Likely an incorrect or invalid review comment.scripts/validate-naming.py (2)
121-193: LGTM!
209-228: LGTM!talos/cluster/authenticate-ghcr-pulls.yaml (1)
1-53: LGTM!talos/cluster/enable-dex-oidc.yaml (1)
1-25: LGTM!talos/cluster/encrypt-ephemeral-volume.yaml (1)
1-24: LGTM!talos/cluster/encrypt-state-volume.yaml (1)
1-22: LGTM!talos/workers/allow-node-exporter-ingress.yaml (1)
1-18: LGTM!talos/workers/allow-nodeport-ingress.yaml (1)
1-13: LGTM!talos/workers/mount-longhorn-data.yaml (1)
11-13: 🎯 Functional Correctness
label-nodes.yamlis present attalos/workers/label-nodes.yaml; the cross-reference is accurate.> Likely an incorrect or invalid review comment.talos/workers/allow-apid-ingress.yaml (1)
1-16: LGTM!talos/workers/allow-cilium-mutual-auth-ingress.yaml (1)
1-23: LGTM! (See the consolidation note left onallow-cilium-health-ingress.yamlregarding merging this rule's port into that file.)talos/workers/allow-cni-vxlan-ingress.yaml (1)
1-12: LGTM! (See the consolidation note left onallow-cilium-wireguard-ingress.yamlregarding merging this rule's port into that file.)talos/workers/allow-hubble-peer-ingress.yaml (1)
1-13: LGTM!
| def count_docs(path): | ||
| """Count YAML documents with any non-comment content (kind-less included).""" | ||
| with open(path, encoding="utf-8", errors="replace") as f: | ||
| text = f.read() | ||
| return sum( | ||
| 1 for chunk in re.split(r"(?m)^---[ \t]*$", text) | ||
| if any(l.strip() and not l.lstrip().startswith("#") for l in chunk.splitlines()) | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Ambiguous variable name l flagged by Ruff.
Rename the loop variable to avoid the E741 lint error.
🧹 Proposed fix
return sum(
1 for chunk in re.split(r"(?m)^---[ \t]*$", text)
- if any(l.strip() and not l.lstrip().startswith("#") for l in chunk.splitlines())
+ if any(line.strip() and not line.lstrip().startswith("#") for line in chunk.splitlines())
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def count_docs(path): | |
| """Count YAML documents with any non-comment content (kind-less included).""" | |
| with open(path, encoding="utf-8", errors="replace") as f: | |
| text = f.read() | |
| return sum( | |
| 1 for chunk in re.split(r"(?m)^---[ \t]*$", text) | |
| if any(l.strip() and not l.lstrip().startswith("#") for l in chunk.splitlines()) | |
| ) | |
| def count_docs(path): | |
| """Count YAML documents with any non-comment content (kind-less included).""" | |
| with open(path, encoding="utf-8", errors="replace") as f: | |
| text = f.read() | |
| return sum( | |
| 1 for chunk in re.split(r"(?m)^---[ \t]*$", text) | |
| if any(line.strip() and not line.lstrip().startswith("#") for line in chunk.splitlines()) | |
| ) |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 113-113: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(path, encoding="utf-8", errors="replace")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 Ruff (0.15.20)
[error] 118-118: Ambiguous variable name: l
(E741)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/validate-naming.py` around lines 112 - 119, The `count_docs` helper
in `validate-naming.py` uses the ambiguous loop variable name `l`, which
triggers Ruff’s `E741` lint error. Rename that inner loop variable in the
`any(...)` expression to a clearer name, keeping the same document-counting
logic and preserving the behavior of `count_docs`.
Source: Linters/SAST tools
github-project-automation
Bot
moved this from 🫴 Ready
to 🏃🏻♂️ In Progress
in 🌊 Project Board
1 participant
Why
Patch files — both the Kustomize overlay patches and the Talos machine-config patches — were named after the resource they patch (
helm-release-patch.yaml×12,sysctls.yaml,kubelet.yaml), which says nothing about what a patch actually does; some files bundled several documents, and three k8s patches lived loose outside apatches/directory.What
Codifies the maintainer-directed convention and applies it everywhere: patches hold one resource/document per file and are named by intent like CR-folder files (
<verb>-<purpose>.yaml). K8s patches live underpatches/(23 renamed, 3 moved, 3 no-op patches deleted); Talos patches keep theirtalos*/layout and get intent names too, with the three multi-document files split one-document-per-file (rule count and the ENOBUFS warning preserved). The naming CI validator enforces all of it. Overlays render byte-identical and Talos config content is verified preserved doc-for-doc — no behavior change.🤖 Generated with Claude Code